-
Notifications
You must be signed in to change notification settings - Fork 3
feat: sim item filters by best valid #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
018c868 to
2c2eb0d
Compare
a8944d4 to
4818f22
Compare
cdaa8aa to
ac4331c
Compare
4818f22 to
dc448ae
Compare
ac4331c to
35e8391
Compare
8b53d65 to
ad9a677
Compare
dc448ae to
4c8148a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last set of nits
768f737 to
236cf11
Compare
| // Peek to perform the balance check for the first tx | ||
| if let Some(first) = items.peek() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only checks the first transaction. if the bundle contains multiple senders, shouldn't we check the first tx balance for each?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read the comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had already read it. I wanted stricter validation here, but I see how making it more strict can discard valid bundles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// We do not check balances after
// the first tx, as they may have changed due to prior txs in the
// bundle.
is there a way we can rewrite this to be more clear?
| // Peek to perform the balance check for the first tx | ||
| if let Some(first) = items.peek() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had already read it. I wanted stricter validation here, but I see how making it more strict can discard valid bundles

SimCache now exposes methods to filter items by the best VALID transaction